Skip to content

Fix DBT converts PERCENTILE_DISC to PERCENTILE_CONT on round-trip - #316

Open
MonkeyCanCode wants to merge 1 commit into
apache:mainfrom
MonkeyCanCode:dbt_from_PERCENTILE_DISC_2_PERCENTILE_CONT
Open

Fix DBT converts PERCENTILE_DISC to PERCENTILE_CONT on round-trip#316
MonkeyCanCode wants to merge 1 commit into
apache:mainfrom
MonkeyCanCode:dbt_from_PERCENTILE_DISC_2_PERCENTILE_CONT

Conversation

@MonkeyCanCode

Copy link
Copy Markdown
Contributor

Summary

Currently _extract_agg_info from https://github.com/apache/ossie/blob/main/converters/dbt/src/ossie_dbt/expression_utils.py#L96 is parse both exp.PercentileCont and exp.PercentileDisc into the same (agg_type, bare_col, percentile) which dropped the discrete flag. The rebuild later on didn't set use_discrete_percentile (which default to False) from https://github.com/apache/ossie/blob/main/converters/dbt/src/ossie_dbt/osi_to_msi.py#L287. Thereforce, a later MSI → OSI pass emits PERCENTILE_CONT instead of honoring original PERCENTILE_DISC when in-used.

To fix this problem, I added one extra field use_discrete_percentile in _extract_agg_info as return value.

Here is the local test validation:

➜  dbt git:(dbt_from_PERCENTILE_DISC_2_PERCENTILE_CONT) ✗ uv run pytest
==================================================================================================================================================== test session starts =====================================================================================================================================================
platform darwin -- Python 3.11.13, pytest-9.1.1, pluggy-1.6.0
rootdir: /Users/yong/Desktop/GitHome/ossie/converters/dbt
configfile: pyproject.toml
testpaths: tests
plugins: syrupy-5.5.3
collected 100 items

tests/test_msi_to_osi.py ..................................................................                                                                                                                                                                                                                            [ 66%]
tests/test_osi_to_msi.py ..................................                                                                                                                                                                                                                                                            [100%]

-------------------------------------------------------------------------------------------------------------------------------------------------- snapshot report summary ---------------------------------------------------------------------------------------------------------------------------------------------------
5 snapshots passed.
==================================================================================================================================================== 100 passed in 0.36s =====================================================================================================================================================

Related Issues

Checklist

Specification

  • Spec changes are included in core-spec/ and follow the existing structure
  • Spec changes have been discussed on the mailing list or in a linked issue
  • Breaking changes to the spec are clearly called out in the summary

Ontology

  • Ontology changes in ontology/ are consistent with spec changes
  • New or modified terms are defined and documented

Converters

  • Converter logic in converters/ is updated to reflect spec or ontology changes
  • New converters include tests under the converter's test directory

Validation

  • Validation rules in validation/ are updated if the spec changed
  • New validation cases are covered by tests

Documentation

  • docs/ is updated to reflect any user-facing changes
  • New features or behaviors are documented with examples where appropriate
  • CONTRIBUTING.md is updated if the contribution process changed

Examples

  • examples/ are added or updated for any new spec constructs or converter support

Tests

  • All existing tests pass (pytest / CI green)
  • New functionality is covered by tests

Compliance

  • ASF license headers are present on all new source files
  • No third-party dependencies are added without PMC/IPMC approval

@MonkeyCanCode

Copy link
Copy Markdown
Contributor Author

@QMalcolm mind take a look?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant